Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rustbuild: Rewrite the configure script in Python #44107

Merged
merged 1 commit into from
Aug 28, 2017
Merged

rustbuild: Rewrite the configure script in Python #44107

merged 1 commit into from
Aug 28, 2017
+653 −1,217

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Aug 26, 2017 •
edited
Loading

This commit rewrites our ancient ./configure script from shell into Python.
The impetus for this change is to remove config.mk which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through config.toml.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a config.toml which is based off config.toml.example.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@Mark-Simulacrum
Copy link
Member

I believe this will also close #43295, #42255, and #38058. Could you verify and update the description?

I've looked through the code and everything looks good to me. If you'd like a rubber stamp approval, feel free to r=me -- I'm not familiar with Python though so can't really review code quality there.

zackmdavis
zackmdavis reviewed Aug 26, 2017
View reviewed changes
configure Outdated

p("processing command line")
i = 1
while i < len(sys.argv):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use argparse (or optparse if we care about supporting the dead hand of Python 2.6) rather than rolling our own?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible, yes, but I'm pretty unfamiliar with both (I barely know python) and would prefer to focus on compatibility for now

@alexcrichton
Copy link
Member Author

Thanks for reviewing @Mark-Simulacrum!

I think I'll hold off on merging this until after beta branches, no need to land a rewrite that has a high likelihood for regressions just before!

@alexcrichton alexcrichton force-pushed the no-shell-configure branch 3 times, most recently from ecc3451 to 1db05ac Compare August 27, 2017 01:45
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

📌 Commit 1db05ac has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

⌛ Testing commit 1db05acbfa2bbe241323b3c7e98ed2407a921b60 with merge a10b0ddf528dd8e29853347fa20ca8c016d573d0...

@bors
Copy link
Contributor

bors commented Aug 27, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

📌 Commit e206216 has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 27, 2017

⌛ Testing commit e2062165d58211ff81c10bc9332e14eb68c6970b with merge f52c915024bac5dbab77ecb715c445cfe7a40dca...

@bors
Copy link
Contributor

bors commented Aug 27, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

Probably a matter of copying the config.toml.example into dist.

00:10:05] configure: processing command line
[00:10:05] configure: 
[00:10:05] configure: rust.debug-assertions := True
[00:10:05] configure: build.vendor         := True
[00:10:05] configure: build.submodules     := False
[00:10:05] configure: llvm.assertions      := True
[00:10:05] configure: build.locked-deps    := True
[00:10:05] configure: llvm.ccache          := sccache
[00:10:05] configure: build.openssl-static := True
[00:10:05] configure: build.build          := x86_64-unknown-linux-gnu
[00:10:05] configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--enable ...
[00:10:05]     for line in open(rust_dir + '/config.toml.example').read().split("\n"):
[00:10:05] IOError: [Errno 2] No such file or directory: '/checkout/obj/build/tmp/distcheck/config.toml.example'
[00:10:05] 
[00:10:05] 
[00:10:05] command did not execute successfully: "./configure" "--build=x86_64-unknown-linux-gnu" "--enable-sccache" "--disable-manage-submodules" "--enable-locked-deps" "--enable-cargo-openssl-static" "--enable-debug-assertions" "--enable-llvm-assertions" "--enable-vendor"
[00:10:05] expected success, got: exit code: 1

@alexcrichton
rustbuild: Rewrite the configure script in Python
a9b0a7b 
This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes rust-lang#40730
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 28, 2017

📌 Commit a9b0a7b has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 28, 2017

⌛ Testing commit a9b0a7b with merge 469d147...

bors added a commit that referenced this pull request Aug 28, 2017
@bors
Auto merge of #44107 - alexcrichton:no-shell-configure, r=Mark-Simula…
469d147 
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
@bors
Copy link
Contributor

bors commented Aug 28, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

@bors: retry

@bors
Copy link
Contributor

bors commented Aug 28, 2017

⌛ Testing commit a9b0a7b with merge a0c3bd2...

bors added a commit that referenced this pull request Aug 28, 2017
@bors
Auto merge of #44107 - alexcrichton:no-shell-configure, r=Mark-Simula…
a0c3bd2 
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
@bors
Copy link
Contributor

bors commented Aug 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing a0c3bd2 to master...

@bors bors merged commit a9b0a7b into rust-lang:master Aug 28, 2017
@bors bors mentioned this pull request Aug 28, 2017
Merged
@alexcrichton alexcrichton mentioned this pull request Aug 30, 2017
Closed
@alexcrichton alexcrichton deleted the no-shell-configure branch August 30, 2017 14:30
@alexcrichton alexcrichton mentioned this pull request Nov 6, 2017
Closed
@o01eg o01eg mentioned this pull request Nov 29, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zackmdavis zackmdavis zackmdavis left review comments

Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexcrichton @rust-highfive @Mark-Simulacrum @bors @zackmdavis @nikomatsakis